perf(server): resuming the thread list takes one fewer round trip by unmanbearpig · Pull Request #21082 · pingdotgg/t3code

unmanbearpig · 2026-10-18T03:57:28Z

Marked thread and shell subscriptions sent synchronized in a separate RPC chunk. Effect RPC waits for each chunk ACK, so clients paid another round trip before catch-up completed. Shell resume also acknowledged metadata and replay batches separately.

Append completion to the initial chunk. Thread prefixes are already eager; marked shell resumes collect the eligible replay before sending metadata, deltas and completion together. Clients omitting the marker keep the existing streaming path. Prefix references are released after delivery, and the persisted live tail still backfills from the captured high-water mark.

Verification on the maintainer head:

58 scoped tests pass across ws.test.ts, rpcInitialItems.memory.test.ts, ShellStream.test.ts, and ThreadStream.test.ts; server typecheck and targeted lint/format pass.
Replaced the PR-specific shell suite and extra memory-fixture assertions with focused cases in the existing WebSocket suite: 42 original directly affected tests → 21 final tests (14 existing, seven added). Six new cases fail immediately against main; legacy no-marker compatibility passes on both.
Two independent full-diff review rounds by Claude Opus 5.5 and Codex GPT-6 Sol found no confirmed code blockers. Memory measurements below cover the remaining evidence question.
Authenticated JSON RPC against an isolated real server/SQLite fixture, comparing exact main production with this head. Three trials per path, 100 ms client ACK delay, 1,000 distinct projects with 1 KiB script commands and 1,000 small update events:

Shell subscription	Main median	This head median	Main → head chunks through completion
Snapshot	268.0 ms	187.1 ms	2 → 1
Resume, no new events	220.8 ms	124.9 ms	2 → 1
Resume, one event	331.4 ms	120.2 ms	3 → 1
Resume, 1,000 events	723.1 ms	544.3 ms	4 → 1

These are local fixture measurements with delayed ACKs, not physical network RTT or end-user UI timings. The one-event and large-replay cases remove more than just the final marker ACK because metadata/replay now share the prefix.

Memory/projection expansion: 1,000 distinct project shells, each with a 1 or 8 KiB script command written before the cursor, followed by 1,000 small update events (74,890 stored payload bytes). Replay coalescing cannot collapse distinct project IDs. The 8 KiB case serializes about 17 MiB through completion; main splits it, this head sends one ~17 MiB first chunk. Cold snapshots already send a ~17 MiB first chunk on both main and this head. Memory sampling is every 10 ms; the client holds the completion chunk for 100 ms, then closes; the server is measured again after GC. Single runs per cell, no universal memory claim.

Fixture / subscriptions	Peak heap MiB, main → head	Peak RSS MiB, main → head	Post-GC heap MiB, main → head
memory, 1 KiB scripts / 1	239.1 → 252.9	554.8 → 558.4	180.9 → 181.0
memory, 1 KiB scripts / 4	329.8 → 358.0	620.1 → 655.4	181.8 → 181.5
memory, 8 KiB scripts / 1	387.6 → 397.7	711.9 → 750.7	201.5 → 201.4
memory, 8 KiB scripts / 4	631.9 → 538.5	952.0 → 1093.3	202.3 → 202.0
snapshot, 8 KiB scripts / 1	381.5 → 383.1	753.4 → 742.8	200.9 → 200.6
snapshot, 8 KiB scripts / 4	494.6 → 442.2	954.3 → 928.4	201.5 → 201.2

The large four-client replay raises observed peak RSS by 141.3 MiB, while sampled peak heap is lower. Head resume is also above head cold snapshot (538.5 vs 442.2 MiB heap, 1093.3 vs 928.4 MiB RSS); main resume is above main cold snapshot too (631.9 vs 494.6 MiB heap). Post-GC heap stays around 202 MiB. An unrelated server.getConfig RPC sent after the first chunk, while its ACK is held, succeeds during the large four-client run: maximum observed response is 1436.5 → 1713.9 ms; the equivalent head cold snapshot is 1702.3 ms and main cold snapshot is 1919.4 ms. These costs matter alongside the saved ACKs. No retained heap after client close and forced GC, OOM, or protocol timeout was observed in these runs. Larger projected strings and more clients remain unbounded.

Exact comparison: main 49b0f96 versus maintainer head 8627f1f. The baseline loads those two production files from main with a Node module hook, leaving checkout and all other code identical.

Not verified: macOS, Windows, iOS, Android, Electron desktop, real relay/tunnel latency, or visual client interactions. This is a server transport change with no UI changes; screenshots do not demonstrate chunk/ACK behavior. The 1,000-event/8 MiB stored-payload eligibility limits do not bound projected bytes or universal process memory.

Original implementation and performance investigation by @unmanbearpig in #21082; contributor commits preserved.

Maintainer takeover: Codex GPT-6 Astra (tests), GPT-6.1 Sol (implementation and integration), GPT-6 Sol and Claude Opus 5.5 (adversarial review), through T3 Code.

macroscopeapp · 2026-10-18T03:59:52Z

Approvability

Verdict: Approved at 84e16c2

Macroscope's review found this PR approvable — This is a focused server-side performance change that coalesces existing reconnect data with its completion marker while preserving ordering and non-marker behavior. Replay buffering is bounded by existing safeguards and supported by extensive lifecycle and chunking tests, with no new capability, schema, deployment, or sensitive-area changes.

^{You can add or adjust custom eligibility rules. Learn more.}

coderabbitai · 2026-10-18T04:06:23Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.config.ts
Review profile: CHILL
Plan: Advanced
Run ID: 79363b09-f716-48ce-86cb-d7fbc7021c18

📥 Commits

Reviewing files that changed from the base of the PR and between 84e16c2 and 8627f1f.

📒 Files selected for processing: 2

apps/server/src/ws.test.ts
apps/server/src/ws.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

📝 Walkthrough

Thread and shell subscriptions now include requested completion markers in initial RPC delivery. Shell resume paths collect bounded replay with the snapshot and marker when requested, then continue live delivery from the replay high-water sequence. Tests cover marker delivery, replay without a marker, failure, and interruption.

Changes: Initial stream delivery

Layer / File(s)	Summary
Initial item batching `apps/server/src/rpcInitialItems.ts`	`rpcInitialItems` accepts an optional completion item and appends it to the initial items when defined.
Thread and shell subscription delivery `apps/server/src/ws.ts`, `apps/server/src/ws.test.ts`	Thread snapshot and replay paths, and shell snapshot paths, pass optional markers through initial delivery. Tests check that the marker appears in the initial RPC chunk and that live events follow acknowledgement.
Shell resume replay `apps/server/src/ws.ts`, `apps/server/src/ws.test.ts`	When a marker is requested, bounded replay is combined with snapshot items and the marker in the initial chunk. Tests also cover replay without a marker, failure, and interruption.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ShellSubscription
  participant ReplayStream
  participant RPCInitialItems
  participant RPCClient
  ShellSubscription->>ReplayStream: Collect bounded replay to high-water sequence
  ShellSubscription->>RPCInitialItems: Provide snapshot items, replay, and optional marker
  RPCInitialItems->>RPCClient: Send initial chunk
  RPCClient->>ShellSubscription: Acknowledge initial chunk
  ShellSubscription->>RPCClient: Send live events from high-water sequence

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 8627f

This change sends the requested synchronization marker with the initial catch-up data, which saves one network round trip when resuming thread and thread-list streams. No merge-blocking risk was identified, and the author reports that the targeted tests pass.

Security architecture risk: 🔵 Low · up to 8627f

This changes how catch-up updates are delivered, not who can access them. Existing access checks and replay limits remain. Combining updates increases temporary memory use, and large or concurrent resumes have not been fully characterized.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

inferred — The changed resource exposure is catch-up preparation and response buffering in the existing server process. An authenticated caller with orchestration-read scope can select the marker-bearing path. The reviewed change does not establish access to additional identities, data stores, tools, or privileged operations.

Trust Boundaries and Controls

observed — WebSocket upgrade authentication precedes RPC server construction. Both production subscriptions require orchestration-read scope, and authorization rejects calls missing their required scope before invoking the handler. These controls are unchanged by the full PR comparison.

Resilience and Maintainability Implications

observed — The thread producer delegates to the existing bounded event-sink stream, which uses the shared subscribe-before-replay helper. Its cursor, retention budget, and subscription scope are created for each execution. This supports independent subscription ownership and recovery across the initial-to-live transition; the added tests do not simulate concurrent writes or repeated subscriptions.

Hardening Proposals

proposed — For stronger resource containment, consider a projected-byte budget for marker-bearing shell replay, with snapshot fallback or incremental delivery when exceeded. Characterize peak memory under large current shells and concurrent resumes before selecting a limit; the existing stored-payload budget should not be treated as a process-memory guarantee.

Pre-merge checks: 4

Check name	Status	Explanation
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Title check		The title clearly and concisely describes the main performance change: thread-list resumption requires one fewer round trip.
Description check		The description provides detailed problem, change, scope, verification, benchmark, memory, compatibility, and limitation information. It does not use the template headings or provide a direct maintain…

Full details: Description check

Explanation

The description provides detailed problem, change, scope, verification, benchmark, memory, compatibility, and limitation information. It does not use the template headings or provide a direct maintainer-approval comment link, but the required content is otherwise substantially covered.

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

juliusmarminge · 2026-10-19T04:22:50Z

Taking over this one. I’ll reduce the tests, rebuild the batching change, and verify replay memory and reconnect latency before pushing to this branch.

…-completion

hieu-lee · 2026-11-01T23:33:47Z

Profiling follow-up: #28931 bounds the settled-timestamp cutoff allocation inside shell trimming. It complements the roundtrip reduction here rather than replacing it. Its description explicitly limits the allocation/no-slowdown result to the measured real-index-shaped workload and discloses larger oldest-first CPU regressions.

unmanbearpig added 3 commits October 18, 2026 00:37

perf(server): batch thread catch-up completion with data

f6e1687

perf(server): batch shell catch-up completion with data

8e02602

test(server): cover eager shell replay lifecycle

84e16c2

github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 18, 2026

macroscopeapp Bot approved these changes Oct 18, 2026

View reviewed changes

juliusmarminge self-assigned this Oct 19, 2026

juliusmarminge added 2 commits October 19, 2026 04:22

Merge remote-tracking branch 'origin/main' into perf/upstream-catchup…

dd81457

…-completion

test(server): focus catchup completion regressions

8627f1f

t3dotgg mentioned this pull request Oct 25, 2026

perf(shell): new sessions resume the cached thread list instead of downloading it #24704

Closed

This was referenced Oct 27, 2026

perf: reduce remote reconnect and command delays #25754

Merged

fix(server): preserve reconnect sync progress on unreliable networks #26422

Closed

hieu-lee mentioned this pull request Nov 1, 2026

perf(server): bound settled-shell cutoff allocation #28931

Open

Navigation Menu

Repository navigation

perf(server): resuming the thread list takes one fewer round trip - #21082

perf(server): resuming the thread list takes one fewer round trip#21082
unmanbearpig wants to merge 5 commits into
pingdotgg:mainfrom
unmanbearpig:perf/upstream-catchup-completion

unmanbearpig commented Oct 18, 2026 •

edited by juliusmarminge

Loading

Uh oh!

macroscopeapp Bot commented Oct 18, 2026

Uh oh!

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading

Sequence Diagram(s)

Uh oh!

juliusmarminge commented Oct 19, 2026

Uh oh!

hieu-lee commented Nov 1, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

unmanbearpig commented Oct 18, 2026 • edited by juliusmarminge Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

macroscopeapp Bot commented Oct 18, 2026

Approvability

Uh oh!

coderabbitai Bot commented Oct 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Sequence Diagram(s)

Uh oh!

juliusmarminge commented Oct 19, 2026

Uh oh!

hieu-lee commented Nov 1, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

unmanbearpig commented Oct 18, 2026 •

edited by juliusmarminge

Loading

coderabbitai Bot commented Oct 18, 2026 •

edited

Loading